Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature to allow ceryx to be used as data by-pass proxy #89

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

w4tsn
Copy link

@w4tsn w4tsn commented Feb 25, 2021

Hello there! I'm happy user of ceryx for around a year now and I've implemented a small set of features for a specific use-case at my startup: using ceryx as configurable proxy to by-pass a cluster application for database access.

The following merge request is rather big, but all the changes are important to enable this use-case / feature / pattern. Let me explain to you what is happening. I hope you can appreciate the additions and I'm happy to make adjustments wherever necessary.

The base material explaining the use-case on a rather theoretical basis is a document from the Eclipse Ditto documentation, Data By-Pass Pattern. The basic idea is to place a random UUID with a "hidden" source URL together with authorization information in ceryx and return a 303 to the client which then is able to get a database query executed by calling ceryx with the UUID, without ever seeing the actual query or credentials. The environment is a cluster application which is not suited to pipe big amounts of data like in a database query.

To enable this I've added several things which are all explained in more detail in the README.

  • Ability to use arbitrary strings as source
  • Ability to use request uri params for targets
  • Ability to provide headers for ceryx calling the target, e.g. for Authorization or Host headers
  • Ability to provide a TTL to automatically clean up and invalidate routes after a certain amount of time

As said previously I'm using this modified Ceryx for about a year now and I think these features could benefit more people out there.

Tell me what you think! :)

More ideas:

  • Ceryx could execute queries in advance, cache them and discard after TTL runs out

In preparation of a larger merge request which adds more ways on
source matching the variables indicating a host match are renamed to the
more generic term source.
In case of a proxy request the headers are hidden for the client calling
the proxy.

In case of the redirect the headers are added before sending the
redirect response back to the client.
It is now possible to add a ttl setting in seconds to the settings of a
route which will set a redis EXPIRE tag on the respective route.

The default is persistent.
The proxy now first tries to find a matching route based on the host and
if this does not yield a result it will next try to find a route based
on the request_uri (without leading slash).

This commit depends on the presence of feature/headers.
In newer versions or some environments it may occur that the responder
API complains about a missing, explicit default route key.
When using redis EXPIRE or any other automated cleaning utility it may
happen that a hash is not fully removed. If a new hash is set at the
same key and there are null values the old stored values may be reused.
@mgaerber
Copy link

👍

@parisk
Copy link
Contributor

parisk commented Feb 26, 2021

Hi @w4tsn thanks a lot for your proposal and your kind works. We will get back on this next week, in order to review the use case, semantics and implmentation.

@parisk parisk changed the base branch from master to add-request_uri-source March 6, 2021 17:57
@parisk parisk changed the base branch from add-request_uri-source to master March 6, 2021 17:59
Copy link
Contributor

@parisk parisk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! 👋

I managed to find an hour during the weekend to review this Pull Request and think about the appropriate direction it should follow for the project. I also pushed a couple of commits, simplifying the documentation (https://github.com/sourcelair/ceryx/pull/89/commits/63ac665106f9050352552ba5ade2ec126030f5ec) and formatting code (https://github.com/sourcelair/ceryx/pull/89/commits/75ed15ae73df8d4f4f62434ce9803683c37da3b7).

First, we should abstract the behaviour around target headers and URL parameters. We have a great opportunity here to pave a straightforward path towards general header and URL parameter manipulation, in addition to just adding values.

Next, I was thinking of ways to keep code and flows as simple as possible — treat special cases as normal ones. In that spirit we should, we can avoid JSON encoding and decoding by using an additional top-level key and reducing multiple routing attempts to one, by leveraging my first point.

To do

To sum things up, we need a few things to get this merged

  • Move headers from settings to top-level key.
    redis.prefix .. ":headers:" .. source
  • Replace string values in headers, with an object:
    {"name": "header-name", "action": "set", "value": "desired-header-value"}
  • Move URL parameters to top-level key.
    redis.prefix .. ":parameters:" .. source
  • Use objects for URL parameters, just like headers:
    {"name": "parameter-name", "action": "set", "value": "desired-parameter-value"}

🙏 Once again, thanks a lot for your contribution @w4tsn. We really appreciate it and can't wait to get this merged upstream.

@w4tsn
Copy link
Author

w4tsn commented Apr 29, 2021

After thinking about this a bit more: I'm totally with you regarding your opinions and ideas on where to go from here but honestly I believe that all those requests is actually killing this MR because it extends the scope to something that I think is worth at least one more MR. Do you see any way of getting this MR merged and starting from there to tackle the refactor and new features?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants